Skip to content

fix(memories): record the resolved path, not the raw pointer (FT-MR11) - #1

Merged
mcp-tool-shop merged 2 commits into
mainfrom
fix/index-gen-resolved-path
Sep 7, 2026
Merged

fix(memories): record the resolved path, not the raw pointer (FT-MR11)#1
mcp-tool-shop merged 2 commits into
mainfrom
fix/index-gen-resolved-path

Conversation

@mcp-tool-shop

Copy link
Copy Markdown
Member

What was wrong

generateIndex resolved each MEMORY.md reference correctly — trying the store directory, then its parent — and then discarded the result, storing ref.path verbatim (index-gen.ts entry builders).

The canonical store writes pointers as memory/foo.md, where memory/ is a namespace label for the store, not a subdirectory of it. So the CLI's rewritePathsAbsolute re-applied the prefix against the store root alone and emitted a doubled …/memory/memory/foo.md.

Why it mattered

On the canonical store this left 420 of 492 published entries (85%) pointing at files that do not exist.

The UserPromptSubmit hook reads that published index on every prompt in every session. When it hands a session a path that isn't there, the session either gets nothing or paraphrases from the one-line summary — which is exactly what the store's own rule forbids ("open the file pointer before acting, do not paraphrase from the summary"). This was silently degrading memory recall everywhere.

The fix

Entries now record the location that actually resolved, relative to the store root with POSIX separators. Both of the store's drifted layouts (274 flat files + 70 nested) resolve, and no memory file has to move — the store is not version-controlled, so not touching it was the low-risk path.

Store-relative rather than absolute keeps the on-disk index portable, which is the contract rewritePathsAbsolute depends on.

Acceptance test

GLOBAL (the hook reads this):  492/492 resolve   BROKEN: 0
STORE (on-disk index)       :  492/492 resolve   BROKEN: 0

Was 72/492. The 59 genuinely-nested entries still resolve correctly.

Why the tests missed it

The original fixture puts MEMORY.md above its memory/ directory, so every ref matched on the first base and the parent-base fallback was never exercised. That is how this reached production.

fixtures/flat-store/ pins the real shape — MEMORY.md inside the store, one flat topic, one genuinely-nested topic — and asserts every entry resolves once joined onto the store root, mirroring the live acceptance check.

Also in this PR

DEFAULT_STORE was an absolute literal containing a username, so the shipped default resolved on exactly one computer and leaked that username into a public package. Now derived from homedir(), matching defaultDest() directly below it. Remaining username literals in repo instructions and test data are gone too.

identity-scan reads RESULT CLEAN; it previously reported HIT 12 on main.

Verification

  • npm run verify383 tests across all four packages, 0 fail, exit 0
  • identity-scan.pyRESULT CLEAN
  • loadout-os refresh — exit 0, no andon halt, live index repaired

Not in scope

Publishing to npm is the Director's call, so the version is unbumped and the entry sits under [Unreleased]. Until a release ships, the global loadout-os binary (v1.0.2) still carries the bug — the live index was repaired by running this branch's build directly.

🤖 Generated with Claude Code

mcp-tool-shop and others added 2 commits September 7, 2026 08:29
`generateIndex` resolved each MEMORY.md reference correctly — trying the store
directory, then its parent — and then discarded the result, storing `ref.path`
verbatim. The canonical store writes pointers as `memory/foo.md`, where
`memory/` is a namespace label for the store rather than a subdirectory of it,
so the CLI's `rewritePathsAbsolute` re-applied the prefix against the store root
alone and emitted a doubled `…/memory/memory/foo.md`.

On the canonical store that left 420 of 492 published entries (85%) pointing at
files that do not exist. The UserPromptSubmit hook reads that published index on
every prompt, so every session was silently handed dead paths and fell back to
paraphrasing one-line summaries — precisely what the store's own rule forbids.

Entries now record the location that actually resolved, relative to the store
root with POSIX separators, so both of the store's drifted layouts resolve and
no memory file has to move. Store-relative rather than absolute keeps the
on-disk index portable, which is the contract `rewritePathsAbsolute` depends on.

Live index: 72/492 -> 492/492.

The original fixture put MEMORY.md *above* its `memory/` directory, so every ref
matched on the first base and the parent-base fallback was never under test —
which is how this reached production. `fixtures/flat-store/` pins the real
shape, with one flat topic and one genuinely-nested topic, and asserts every
entry resolves once joined onto the store root.

Also drop the hardcoded home directory from `DEFAULT_STORE`: it was an absolute
literal containing a username, so the shipped default resolved on exactly one
computer and leaked that username into a public package. Now derived from
`homedir()`, matching `defaultDest()` directly below it. The remaining username
literals in repo instructions and test data are gone too; `identity-scan` reads
RESULT CLEAN where it previously reported HIT 12.

Verify: 383 tests across all four packages, 0 fail, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's `npm audit --audit-level=moderate` step went red on three high-severity
brace-expansion DoS advisories (GHSA-3jxr-9vmj-r5cp, GHSA-mh99-v99m-4gvg,
GHSA-rgw5-rvv9-x895). Not introduced here — the advisories postdate main's last
green run on 2026-06-16, so main is red on this today too; it surfaced on this
PR because this is the first run since.

Dev-only transitive path: c8 -> test-exclude -> minimatch -> brace-expansion.
`npm audit fix` resolves it inside the existing minimatch range, so the delta is
five lockfile lines and one package. Audit now reports 0 vulnerabilities.

Verify: 383 tests across all four packages, 0 fail, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mcp-tool-shop
mcp-tool-shop merged commit 823533e into main Sep 7, 2026
3 checks passed
@mcp-tool-shop
mcp-tool-shop deleted the fix/index-gen-resolved-path branch September 7, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant